Adds tests for isnan().#466
Conversation
| ... | ||
| #--- end | ||
|
|
||
| # llvm/llvm-project#141089 |
There was a problem hiding this comment.
the issue you linked is closed llvm/llvm-project#141089 are you sure this should still xfail for clang-vulkan?
There was a problem hiding this comment.
Clang-Vulkan XFAIL removed.
| # llvm/llvm-project#141089 | ||
| # XFAIL: Clang-Vulkan | ||
|
|
||
| # https://github.com/llvm/llvm-project/issues/145571 |
There was a problem hiding this comment.
We added the isnan emulation see https://github.com/llvm/llvm-project/pull/157505/files this should work for clang.
There was a problem hiding this comment.
I don't have an nVidia GPU, so I suppose I can't test for DirectX-NV?
There was a problem hiding this comment.
We don't run the nvidia gpu runner yet on the PR runs. and the only way to manually invoke it would be if you had a branch on this repository. There doesn't apear to be anyway to do that with a fork. That said I feel confident that if you are seeing the emulation passing on amd\intel for sm 6.8 and lower then it should pass for nvidia too since we won't be emitting the opcode that would blow things up.
There was a problem hiding this comment.
If you add a test-all label to the PR then it should run against all the machines in the offload tester lab.
https://github.com/llvm/offload-test-suite/blob/main/docs/CI.md
If this doesn't work for PRs from forks we should update these docs!
damyanp
added
the
test-all
|
All the tests are failing could you update the branch? |
|
I'm just now getting back to this and still get the original error on my side. |
danbrown-amd
force-pushed
the
isnan
branch
2 times, most recently
from
6e5a2e7 to
e03662e
Compare
|
@farzonl would you please rerun the tests so I can see whether I still need XFAIL for NV/QC? |
farzonl
left a comment
farzonl
left a comment
There was a problem hiding this comment.
test LGTM.
Will just want to confirm test passes across vendors before merging.
If you see issues feel free to file issues and add vendor specific xfails for now.
|
On the last test run, 8 tests passed, 9 failed with errors in previously existing tests, and 6 did not run to completion. I rebased and updated on my end, with the only change in the added tests being that I removed some obsolete comment lines. |
|
@danbrown-amd is this ready to be merged? |
|
Sorry for letting this languish, but it looks like we forgot to merge it! I'm going to update the branch to make sure the test still pass and merge if so. |
it's been moved to DispatchParameters and matches the default anyway Co-authored-by: Justin Bogner <mail@justinbogner.com>
4 participants
Test values taken from the isinf() tests; others may be relevant.